Add optional rerun debug view of placement validation - #984
Conversation
alexmillane
left a comment
There was a problem hiding this comment.
Thanks for adding this functionality! Looks pretty good.
See suggestions below!
64f02fe to
5c0022c
Compare
Greptile SummaryThe PR adds an opt-in Rerun visualization and recording path for placement-validation candidates, including cuRobo reachability details.
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking robustness issue when the optional viewer launcher is unavailable. Placement and validator behavior remain intact, but enabling the live debug view can abort ObjectPlacer construction because viewer-process launch errors are not converted into the intended warning-and-continue behavior. Files Needing Attention: isaaclab_arena/relations/placement_visualizer.py Important Files Changed
Sequence DiagramsequenceDiagram
participant Config as Env graph / ObjectPlacerParams
participant Placer as ObjectPlacer
participant Viz as PlacementRerunVisualizer
participant Validators as Placement validators
participant IK as cuRobo reachability
Config->>Placer: debug visualization settings
Placer->>Viz: create viewer and/or file sink
Placer->>Viz: log candidate layout batch
Placer->>Validators: validate inexpensive checks
Placer->>Validators: validate filtered expensive checks
Validators->>IK: solve grasp feasibility
IK->>Viz: log robot, grasps, and IK errors
Placer->>Viz: log per-layout verdicts
Reviews (1): Last reviewed commit: "refactor" | Re-trigger Greptile |
|
@/tmp/body.txt |
Placement solving is sim-free, so its candidate layouts have been hard to inspect without starting Isaac Sim. Opting in via ObjectPlacerParams now streams every candidate to a Rerun viewer (or an .rrd, for headless runs). - Add PlacementRerunVisualizer: one process-wide view that draws each candidate's object boxes and the checks that accepted or rejected it, one frame per candidate on a `candidate` timeline. - Turn it on with ObjectPlacerParams.debug_visualize / debug_visualize_rrd_path; ObjectPlacer builds the view before its validators so a check can add its own layer to it. - Let the cuRobo reachability check layer on what only it knows: the robot base frame, the top-down grasps it solved, per-target reachable/unreachable verdicts, and its IK error scalars. - Keep frames aligned across checks: expensive checks only see the candidates that passed the cheap ones, so the placer tells the view which candidates the running check was handed. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Add debug_visualize / debug_visualize_rrd_path to PlacementValidatorSpec so an env's YAML can ask for the Rerun view without editing Python. - Forward both through build_checks_for_placer_params into ObjectPlacerParams. - Turn the view on in the butter_raisin_box scene as a worked example. - Cover the default-off and forwarding paths in the graph-spec tests. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Spawn the Rerun viewer under `setpriv --pdeathsig TERM` instead of `rr.spawn()`, so the kernel closes the window when the run exits. - Isaac Sim's `SimulationApp.close()` ends in `os._exit()`, so no `atexit` hook can do this. - Stops a stale viewer from holding port 9876, which silently made the next run's spawn a no-op. - Wait for the viewer to serve before logging, replacing the readiness wait `rr.spawn()` did. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Take `debug_visualize: true` out of the shipped butter/raisin scene, where it spawned a viewer window on every build, and move the worked example to a test-data graph YAML. - Guard that with a test asserting no versioned env under `isaaclab_arena_environments` asks for it. - Track which candidates each check was run on, so the view no longer draws an expensive check as rejecting a layout it skipped; the summary count now derives from the same record. - Warn when the viewer port is already served, and confirm the spawned viewer is still alive once it answers, so a run cannot silently log into somebody else's window. - Fall back to killing a viewer that ignores SIGTERM instead of raising out of close(). Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Acceptance now follows the placer, which gates layouts on required_checks alone. - A failure the placer does not gate on reads as accepted, naming the advisory check. - Pull the wording out into summarize_candidate_verdict() so the rule is unit-testable. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
- Show the env graph YAML and Python routes in the visualizer module, pointing at the worked example. - Say the same from the reachability check, whose layer only appears once that view is on. - Drop ACCEPTED_COLOR / REJECTED_COLOR, which nothing has ever drawn with. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
Keep ObjectPlacerParams data-only: checks reach the live Rerun view through get_active_placement_visualizer() rather than a field on the params, and the per-batch layout/verdict logging moves off ObjectPlacer onto the visualizer that owns it. Rename debug_visualize_rrd_path to debug_visualize_output_path and the opaque batch "slot" to batch_index, shorten the docstrings that described code living elsewhere, and fix the enabled check name in the worked YAML example (ik_reachability -> ik_reachable), which was being silently dropped as unregistered. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
Say what each piece does in plain terms: shorter constant docs, one straightforward line per function, and the remaining rationale (setpriv, the shared view, the deepcopy guard) stated directly instead of alluded to. Signed-off-by: Xinjie Yao <xyao@nvidia.com>
27cf00e to
b4e2a84
Compare
alexmillane
left a comment
There was a problem hiding this comment.
LGTM
One comment about dealing with the indices.
Summary
Rerun debug view of placement validation
Detailed description
placement_validators.debug_visualize(and/ordebug_visualize_rrd_pathfor headless runs) in an env graph YAML, orObjectPlacerParams(debug_visualize=True).Example viz
In base docker (during solver validation):
https://github.com/user-attachments/assets/d73882ee-ab13-4309-aa7b-f19de10ad13c
In curobo docker (where reachability is enabled)
https://github.com/user-attachments/assets/55d99e37-6eff-4558-8444-e0a54f251f04